Skip to content

Feat/multiselect filter#232

Open
steveops wants to merge 5 commits intodevfrom
feat/multiselect-filter
Open

Feat/multiselect filter#232
steveops wants to merge 5 commits intodevfrom
feat/multiselect-filter

Conversation

@steveops
Copy link
Contributor

@steveops steveops commented Mar 26, 2020

-Add ability to filter by multiple dropdown values

@pull-assistant
Copy link

pull-assistant bot commented Mar 26, 2020 •
edited
Loading

Score: 0.92

Best reviewed: commit by commit

Optimal code review plan (1 warning)

Implement multi-select for dropdown filter

package-lock.json 46% changes removed in Add ability to overr...

     Test filtering with dropdown with multiple selection

     Add ability to override multi-select theme

     Fix tests

     Separate data type from custom rendering

Powered by Pull Assistant. Last update 67ad063 ... 9b78c01. Read the comment docs.

@trafico-bot trafico-bot bot added the 🔍 Ready for Review Pull Request is not reviewed yet label Mar 26, 2020
accesslint[bot]
accesslint bot suggested changes Mar 26, 2020
View reviewed changes
Copy link

@accesslint accesslint bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are accessibility issues in these changes.

@trafico-bot trafico-bot bot added ⚠️ Changes requested Pull Request needs changes before it can be reviewed again and removed 🔍 Ready for Review Pull Request is not reviewed yet labels Mar 26, 2020
@lgtm-com
Copy link

lgtm-com bot commented Mar 26, 2020

This pull request fixes 1 alert when merging c210fc5 into 245afd0 - view on LGTM.com

fixed alerts:

  • 1 for Overwritten property

@vizipi
Copy link

vizipi bot commented Mar 26, 2020 •
edited
Loading

Pull request analysis by VIZIPI

Below you will find who is the most qualified team member to review your code.
This analysis includes his/her work on the code included in this Pull request, in addition to their experience in code affected by these changes ( partly found within the list of potential missing files below )   Feedback always welcome

Reviewers with knowledge related to these changes

Match % Person Commit Count Common Files
100.00 % Darryn Ten 159 10
70.00 % Igor 16 7
40.00 % Bamidele Daniel 4 4

Potential missing files from this Pull request

files commonly committed with a subset of this pr, but not committed this time. (click to collapse)
FilePercentilerate
src/components/TableLinkRow.vue66.67 %2 out of 3 times
src/components/TableRow.vue66.67 %2 out of 3 times
src/components/TableHeaders.vue66.67 %2 out of 3 times
src/components/TableBody.vue66.67 %2 out of 3 times
src/components/TabbedFilters.vue66.67 %2 out of 3 times
src/components/FilterCheckbox.vue66.67 %2 out of 3 times
src/components/FilterDateRange.vue66.67 %2 out of 3 times
src/components/Searching.vue66.67 %2 out of 3 times
src/components/DatePicker.vue66.67 %2 out of 3 times
src/components/FilterNumberRange.vue66.67 %2 out of 3 times

Committed file ranks

(click to expand)
  • 95.45%[src/DataTable.vue]
  • 92.42%[src/components/TableData.vue]
  • 86.36%[src/components/FilterDropdown.vue]
  • 98.48%[package-lock.json]
  • 96.97%[package.json]
  • 78.79%[src/components/Filtering.vue]
  • 68.18%[webpack.config.js]
  • 62.12%[README.md]
  • 68.18%[src/main.js]
  • 77.27%[test/specs/DataTable.spec.js]

    • @trafico-bot trafico-bot bot added the 🔍 Ready for Review Pull Request is not reviewed yet label Mar 29, 2020
    accesslint[bot]
    accesslint bot suggested changes Mar 29, 2020
    View reviewed changes
    Copy link

    @accesslint accesslint bot left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    There are accessibility issues in these changes.

    src/components/FilterDropdown.vue
    <vue-select ref="select" v-model="filter.value" :options="dropdownOptions" v-if="filter.multiple"
    :multiple="true"
    :reduce="item => item.value" placeholder="All"/>
    <select v-else v-model="filter.value" :style="dropdownStyle" :name="`${filter.filter}`">
    Copy link

    @accesslint accesslint bot Mar 29, 2020

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Looks like there's a label missing for this input. That makes it hard for people using screen readers or voice control to use the input.

    @trafico-bot trafico-bot bot added ⚠️ Changes requested Pull Request needs changes before it can be reviewed again and removed ⚠️ Changes requested Pull Request needs changes before it can be reviewed again 🔍 Ready for Review Pull Request is not reviewed yet labels Mar 29, 2020
    @lgtm-com
    Copy link

    lgtm-com bot commented Mar 29, 2020

    This pull request fixes 1 alert when merging f446f6c into 245afd0 - view on LGTM.com

    fixed alerts:

    • 1 for Overwritten property

    @trafico-bot trafico-bot bot added 🔍 Ready for Review Pull Request is not reviewed yet and removed ⚠️ Changes requested Pull Request needs changes before it can be reviewed again labels Mar 30, 2020
    @lgtm-com
    Copy link

    lgtm-com bot commented Mar 30, 2020

    This pull request fixes 1 alert when merging e3ad3b7 into 245afd0 - view on LGTM.com

    fixed alerts:

    • 1 for Overwritten property

    @lgtm-com
    Copy link

    lgtm-com bot commented Apr 11, 2020

    This pull request fixes 1 alert when merging 9b78c01 into 245afd0 - view on LGTM.com

    fixed alerts:

    • 1 for Overwritten property

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Reviewers

    @darrynten darrynten Awaiting requested review from darrynten

    @igorsergiichuk igorsergiichuk Awaiting requested review from igorsergiichuk

    @humanityjs humanityjs Awaiting requested review from humanityjs

    1 more reviewer

    @accesslint accesslint[bot] accesslint[bot] requested changes

    Reviewers whose approvals may not affect merge requirements

    At least 1 approving review is required to merge this pull request.

    Assignees

    No one assigned

    Labels

    🔍 Ready for Review Pull Request is not reviewed yet

    Projects

    None yet

    Milestone

    No milestone

    Development

    Successfully merging this pull request may close these issues.

    1 participant

    @steveops